chore(llmobs): move agent manifest to _dd field#16621
Conversation
Codeowners resolved as |
This comment has been minimized.
This comment has been minimized.
ncybul
added
the
changelog/no-changelog
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 930d14a689
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| metadata_dd = metadata.get("_dd") or {} | ||
| metadata_dd["agent_manifest"] = span._get_ctx_item(AGENT_MANIFEST) |
There was a problem hiding this comment.
_dd metadata before writing manifest
If user-supplied metadata already contains _dd as a truthy non-dict (for example {"_dd": "custom"}), metadata.get("_dd") or {} returns that value and metadata_dd["agent_manifest"] = ... raises a TypeError. _submit_llmobs_span catches TypeError and drops the event, so agent spans are silently lost for this input shape; _dd should be validated/coerced to a dict before mutation.
Useful? React with 👍 / 👎.
brettlangdon
left a comment
brettlangdon
left a comment
There was a problem hiding this comment.
Not sure why I was pulled as codeowner for: tests/contrib/claude_agent_sdk/test_claude_agent_sdk_llmobs.py
might need to update CODEOWNERS
but regardless, this change lgtm
|
/merge |
|
View all feedbacks in Devflow UI.
This pull request is not mergeable according to GitHub. Common reasons include pending required checks, missing approvals, or merge conflicts — but it could also be blocked by other repository rules or settings.
devflow unqueued this merge request: It did not become mergeable within the expected time |
4 participants
Description
This PR nests the agent manifest under the
_ddfield within the metadata which will be filtered out on the FE. The goal is to move all fields that are internally used to this nested scope to avoid clashing with user-submitted metadata.Corresponding PR to update the FE to check for the agent manifest under this new scope.
Testing
Risks
Additional Notes